Skip to content

DO NOT MERGE: Implement new "ip flag" and "tcp flag" primitives.#1621

Open
infrastation wants to merge 1 commit intothe-tcpdump-group:masterfrom
infrastation:ip_tcp_flag
Open

DO NOT MERGE: Implement new "ip flag" and "tcp flag" primitives.#1621
infrastation wants to merge 1 commit intothe-tcpdump-group:masterfrom
infrastation:ip_tcp_flag

Conversation

@infrastation
Copy link
Copy Markdown
Member

This is one of the potential solutions discussed on tcpdump-workers in November 2025 instead of the changes in pull request #1210. Needs some feedback.

@infrastation
Copy link
Copy Markdown
Member Author

One problem with this revision is that the ip flag implementation does not check whether the packet is an IPv4 packet before testing the part of the packet where IPv4 header flags are in an IPv4 packet. This is trivial to fix, but this also indicates a gap in the new tests because the new tests pass. So this will require at least one more revision to address both problems.

@infrastation
Copy link
Copy Markdown
Member Author

This revision gets ip flag right. Apparently, one of the new apply filter tests would normally have failed due to this bug, but I had set the expected results incorrectly, which resulted in a false positive. Which yet another time proves that even seemingly simple changes have potential to be incorrect. To that end, this revision exercises the flags more intensively and the capture files have comments with packet decoding to make it easier to interpret the expected results.

In the scanner define a new "flag" token and in the parser make it a new
non-directional type qualifier, Q_FLAG.

In gencode.c introduce a generic structure to declare flag states and
through gen_scode() delegate Q_FLAG to a new function, gen_flag().  In
the latter transcribe flag states for IPv4 and TCP, validate the
provided proto qualifier and the ID and dispatch the request to one of
the other new functions, gen_ip_flag() and gen_tcp_flag().

Document the new primitives in pcap-filter(7).  Add tests to cover the
new code paths.  The new "afs.pcap" file comprises packets 124~133 from
afs.pcap in tcpdump tests.
@infrastation
Copy link
Copy Markdown
Member Author

Rebased on the current master branch. This revision uses C99 types in the new code, keeps all packet data loads 8-bit, replaces more code with declarations and matches TCP flags using the same style as in gen_load_internal().

This still requires minor corrections in the man page before merging, but otherwise looks nearly ready.

@fxlb
Copy link
Copy Markdown
Member

fxlb commented Mar 4, 2026

I dont like very much -cleared.
Why not
-set/-unset
-on/-off
-yes/-no
-y/-n
?
The last three are shorter.

@infrastation
Copy link
Copy Markdown
Member Author

My message to tcpdump-workers on February 6th among other things explains the choice of string IDs. Because this will have to be fixed and maintained long-term, the design choices should be recorded long-term, so the mailing list is the best place to discuss the syntax. Please make a better suggestion there if you can.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

2 participants